• X
• Mastodon
• LinkedIn
• Facebook

qa/context/bot_capabilities.json (1)

3-5: Redundant entry: both "A" and "a" are listed.

Per the BOT_KNOWN_COMMANDS comment in index.js (line 300), /A is normalized to lowercase by the group guard, so 'a' covers both. Listing both in this capabilities manifest is redundant and may cause confusion in downstream tooling that consumes this file.

Consider removing the uppercase "A" entry.

♻️ Suggested fix

   "commands": [
-    "A",
     "a",
     "admin",

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@qa/context/bot_capabilities.json` around lines 3 - 5, Remove the redundant
uppercase command entry by deleting "A" from the "commands" array in
qa/context/bot_capabilities.json so only "a" remains; this aligns with the
BOT_KNOWN_COMMANDS normalization (see the group guard behavior in index.js) and
prevents duplicate command representations.

scripts/ast_audit.js (2)

9-18: Wrap readdirSync in try/catch for resilience.

Per project guidelines, readdirSync calls should be wrapped in try/catch to handle permission errors or inaccessible directories gracefully.

Based on learnings: "For Runewager test files: wrap readdirSync calls in try/catch."

🛡️ Suggested defensive handling

 function listFiles(dir) {
   const out = [];
-  for (const ent of fs.readdirSync(dir, { withFileTypes: true })) {
+  let entries;
+  try {
+    entries = fs.readdirSync(dir, { withFileTypes: true });
+  } catch (err) {
+    console.warn(`Warning: Could not read directory ${dir}: ${err.message}`);
+    return out;
+  }
+  for (const ent of entries) {
     if (['node_modules', '.git'].includes(ent.name)) continue;
     const p = path.join(dir, ent.name);
     if (ent.isDirectory()) out.push(...listFiles(p));
     else if (/\.(js|ts)$/.test(ent.name)) out.push(p);
   }
   return out;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/ast_audit.js` around lines 9 - 18, The listFiles function should wrap
the fs.readdirSync(dir, { withFileTypes: true }) call in a try/catch to avoid
crashing on permission/inaccessible directories; catch errors around that call
inside listFiles and on error either return an empty array (skip that directory)
or log a warning and continue so the recursion proceeds safely; ensure the catch
only affects the current directory iteration and retains existing behavior of
pushing .js/.ts file paths when readdirSync succeeds.

---

133-133: Expand catch-all regex detection.

Line 133 only checks for the exact pattern '/.*/', but per project guidelines, catch-all patterns include variants like (.*), (.+), (?:.*), ^(?:.*)$, and (.+)?.

Based on learnings: "expand isCatchAllRegexPattern() to detect (.*), (.+), (?:.*), ^(?:.*)$, (.+)? patterns."

♻️ Suggested enhancement

-const fallback = data.actions.find(a => a.pattern.kind === 'regex' && a.pattern.value === '/.*/');
+function isCatchAllRegex(pattern) {
+  if (pattern.kind !== 'regex') return false;
+  const v = pattern.value;
+  // Match common catch-all patterns: /.*/, /(.*)/, /(.+)/, /(?:.*)/, etc.
+  return /^\/\^?\(?[\?:]*[\.\*\+]+\)?\$?\/[a-z]*$/i.test(v);
+}
+const fallback = data.actions.find(a => isCatchAllRegex(a.pattern));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/ast_audit.js` at line 133, The current fallback detection only
matches the exact string '/.*/'; replace the inline equality check used in const
fallback = data.actions.find(...) with a call to a new helper
isCatchAllRegexPattern(patternValue) and implement that helper to recognize
catch-all variants such as /.*/, (.*), (.+), (?:.*), ^(?:.*)$, (.+)? and their
anchored or slash-delimited forms; implement the helper by normalizing the input
(strip surrounding slashes, optional ^/$ anchors, optional non-capturing group
syntax (?:), and optional trailing ?), then test the normalized body against the
core patterns like '.*' or '.+' (and their groupings) so fallback correctly
detects all allowed catch-all regex variants referenced in the review.

index.js (2)

14139-14146: Redundant rollback call.

restoreTestAllStateSnapshot(snapshot) is called twice: once inside the "Rollback + Persist Reports" step (line 14070) and again here (line 14140). While harmless due to idempotency, the second call appears to be defensive redundancy. Consider documenting the intent or removing the duplicate.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@index.js` around lines 14139 - 14146, The duplicate defensive call to
restoreTestAllStateSnapshot(snapshot) should be removed (or explicitly
documented) because it's already invoked in the "Rollback + Persist Reports"
step; update the catch block where restoreTestAllStateSnapshot is invoked again
and instead only set reportData.rollback.restored = true when appropriate or add
a comment explaining idempotency if you intentionally keep it; refer to the
restoreTestAllStateSnapshot function and the reportData.rollback/restored/error
fields to locate and adjust the logic so the rollback is not executed twice.

---

14027-14039: Consider defensive error handling for AST file read.

The synchronous JSON.parse(fs.readFileSync(...)) at line 14029 will throw if the file is missing or malformed. While withStep catches this, a specific error message would improve debuggability.

♻️ Optional: add explicit try/catch

   await withStep('Action/Callback Tests (registered actions + regex samples)', async (step) => {
     const astPath = path.join(__dirname, 'reports', 'ast_audit.json');
-    const ast = JSON.parse(fs.readFileSync(astPath, 'utf8'));
+    let ast;
+    try {
+      ast = JSON.parse(fs.readFileSync(astPath, 'utf8'));
+    } catch (err) {
+      throw new Error(`Failed to load AST audit: ${err.message}`);
+    }
     for (const action of ast.actions || []) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@index.js` around lines 14027 - 14039, The JSON.parse(fs.readFileSync(astPath,
'utf8')) call can throw if the AST file is missing or malformed; wrap the
read+parse for ast (the astPath/ast variable usage inside the withStep callback)
in an explicit try/catch, and on error set step.warning (or step.error if
available), push a clear error entry into reportData.callbackResults (or a
dedicated reportData.errors array), set reportData.status = 'issues' and skip
the action loop (return or continue) so safeInvokeAction is not run against an
undefined ast; this keeps withStep but provides a specific, actionable error
message for debugging.

docs/features/10-deploy-ops.md (1)

139-146: Minor documentation improvement: consider adding retention details.

The rotating-file-stream package supports maxFiles option for retention. The documentation mentions "old files are pruned automatically (maxFiles retention)" but doesn't specify the actual retention count. According to the code snippet from index.js:13736-13784, the maxFiles is set to 14. Consider adding this detail for completeness.

📝 Optional improvement

 ### Rotating log policy for `/testall`

 - `/testall` writes per-step progress and errors to rotating file stream: `data/logs/testall/testall.log`.
 - Rotation triggers: daily interval **or** size threshold (`10M`).
-- Rotated files are gzip compressed and old files are pruned automatically (`maxFiles` retention).
+- Rotated files are gzip compressed and old files are pruned automatically (`maxFiles: 14` retention — ~2 weeks of history).
 - Final `/testall` reports include references to latest + retained rotating log artifacts for audit traceability.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/features/10-deploy-ops.md` around lines 139 - 146, The docs mention
automatic pruning via `maxFiles` but omit the actual retention count; update the
`/testall` rotating log policy description to explicitly state that `maxFiles`
is set to 14 (per the implementation in `index.js` around the
rotating-file-stream setup), e.g., "rotated files are gzipped and pruned with a
retention of 14 files (`maxFiles: 14`)", and ensure this change references the
`/testall` rotating stream configuration so readers can correlate the docs with
the `maxFiles` setting.